Arrayslice #96
Arrayslice #96
Conversation
…uble with other branches
|
The latest commit to this branch includes the following updates: -- the -- added working unit test -- I also added a couple of typemaps in Here's a screenshot of the output this produces:
|
|
Just to update, all tests are passing on my local machines, and the individual tests all pass on travis, but the overall Previously it seems Chris fixed this by doing the python2 and python3 builds in different directories, but that doesn't seem to be working here. Does anybody have any other ideas what might be going on? I'll keep plugging away, but in the meantime the array_slice feature is ready to use per the python snippet above. |
|
Recently I've seen this error message when the real cause was a previously failed build step. I had to search through the logs for "error" and "fail." Looking at the travis log from your latest commit, I see this: |
|
Thanks Chris! Good catch---I had been distracted by a red herring. I appreciate your eagle eye! Now all tests are passing, but the travis build is still exiting with 1. No errors or failures reported in log file---it successfully completes everything, then exits with 1. I've seen this one before too, and can't remember what the workaround or remedy was. Any ideas? |
|
For now I reverted the I have learned that by installing the 'docker' system I can recreate the travis build+test environment on my local machine, which will make it much faster to debug travis problems. I am working on setting that up. When I have made progress debugging travis builds I will add an array-slice unit test to the travis build, but it seems like that could be a separate issue that doesn't need to be a blocker for using the array-slice feature. |
libmeepgeom/array-slice-ll.cpp
Outdated
| double Scale = fmax( fabs(d1[n]), fabs(d2[n]) ); | ||
| if (Scale<1.0e-8) | ||
| continue; | ||
| double RelErr = fabs(d1[n] - d2[n]) / Scale; |
There was a problem hiding this comment.
A more reliable way to compare two floating-point arrays is typically to compute ‖d₁-d₂‖ < reltol * (‖d₁‖+‖d₂‖)/2, where ‖⋯‖ denotes the norm (e.g. the Euclidean/L₂ norm) of the whole array.
This avoids problems with scaling and having to arbitrarily drop values below some threshold. If you want a more strict test that is sensitive to isolated errors in individual values, just use the Chebyshev (L∞) norm for ‖⋯‖.
libmeepgeom/Makefile.am
Outdated
| TESTS = cyl-ellipsoid-ll | ||
|
|
||
| noinst_PROGRAMS = bend-flux-ll | ||
| noinst_PROGRAMS = bend-flux-ll array-slice-ll |
There was a problem hiding this comment.
You're not actually running this test yet in make check, is that right?
| // Typemap suite for array_slice | ||
| // TODO: add (cdouble *, int) version | ||
| %apply (double* INPLACE_ARRAY1, int DIM1) {(double *slice, int slice_length)}; | ||
| %apply int INPLACE_ARRAY1[ANY] { int [3] }; |
There was a problem hiding this comment.
@ChristopherHogan will need to add a higher-level API function (added to the Simulation class, probably) that does the reshape etc automatically, but this is fine for now.
src/array_slice.cpp
Outdated
| /***************************************************************/ | ||
| #define BUFSIZE 1<<16 // use 64k buffer | ||
| if (has_imag) | ||
| { cdouble buffer[BUFSIZE]; |
There was a problem hiding this comment.
In general I would prefer not to stack-allocate here. This is allocating 2¹⁶ * (16 bytes per cdouble) = 1MB for the buffer, which could easily overflow the stack size limit on some systems. (e.g. the default stack limit on Windows is 1MB)
Use new/delete instead.
…ray-slice-ll unit test
|
Fixed errors. Travis build/test now succeeding including |
The primary new routine here is
The implementation is similar to
output_hdf5, but I rearranged the calling convention based on the following rationale:-- the subvolume of interest, and the desired component(s), must always be specified and have no defaults, so those should come first
-- for components, I use a
std::vectorinstead of pointer-plus-integer-valued-size-parameter as inoutput_hdf5-- callers should be able to omit the
field_functionargument to request the identity function-- callers should have the option of (a) passing a caller-allocated array to be filled in with the array-slice data, (b) having the function allocate and return a new array.
For option (a) above I added two new routines:
get_array_slice_dimensions()and
allocate_array_slice_buffer()with self-explanatory calling conventions.
If the caller passes a non-null buffer, it's not obvious how we would check to make sure its the correct size. Some options include
(a) don't check, creating the risk of core dump if the caller passes a buffer that's too small
(b) use a container like
std::vector<cdouble>instead of a bare pointer(c) use some sort of high-level multidimensional array construct that would play nicely with existing python libraries
For the time being I've gone with (a).
Testing: Added
tests/array_slice_test.cppfor testing. This is a modified version ofh5test.cpp. The implementation is not complete, but the idea is to use the same volumes and subvolumes as h5test.cpp and compare the resulting array slices with arrays read in from hdf5 files produced by runningh5test.Usage example: